Add App.focus() method #3034
Add App.focus() method #3034
Conversation
This adds a new focus() method to App that allows programmatically bringing the application into focus. The implementation is platform-specific: - macOS: Uses activateIgnoringOtherApps_ - Windows: Uses Form.Activate() - GTK: No-op (window managers control focus) - Mobile/Web: No-op The method includes documentation noting that programmatically stealing focus is generally considered poor practice and should be used sparingly.
There was a problem hiding this comment.
Hi - thanks for the contribution!
This PR is currently raising failures on CI because of pre-commit and towncrier problems; our contribution guide contains details on what you need to do to correct these problems.
Once those problems are fixed, you'll hit a second problem - testing. You've added a new feature - that means you need to add tests. In this case you'll need a test in the core (which will test the dummy backend), and a test in the testbed. The testbed test will be a bit of a no-op because we give focus to another app; but we can exercise the API to make sure it doesn't raise an error (demonstrating that at least the APIs we're invoking actually exist).
| @@ -352,7 +352,7 @@ def show_about_dialog(self): | |||
| if self.interface.author is None: | |||
| options["Copyright"] = "" | |||
| else: | |||
| options["Copyright"] = f"Copyright © {self.interface.author}" | |||
There was a problem hiding this comment.
Why have you made this change? The copyright is a valid symbol, and it displays correctly in my testing.
| @@ -1020,6 +1020,16 @@ def set_full_screen(self, *windows: Window) -> None: | |||
| # End backwards compatibility | |||
| ###################################################################### | |||
|
|
|||
| def focus(self): | |||
There was a problem hiding this comment.
The methods on this class are sorted; this should be added to the "Window control" section, alphabetically sorted in that section.
| """Bring the application into focus. | ||
| This method will attempt to bring the application window into focus. Note that | ||
| it is generally considered poor practice for applications to steal focus from | ||
| the user, so use this method sparingly. | ||
| This is a no-op on mobile and console platforms. |
There was a problem hiding this comment.
We try not to call out specific platforms in docstrings; we put platform specific notes in the Notes section for the class in question.
| """Bring the application into focus. | |
| This method will attempt to bring the application window into focus. Note that | |
| it is generally considered poor practice for applications to steal focus from | |
| the user, so use this method sparingly. | |
| This is a no-op on mobile and console platforms. | |
| """Force the app to be the currently active app on the user's desktop. | |
| This request will not be honoured by all platforms; see the :ref:`platform notes | |
| <app-notes>` for details. | |
| This method should be used sparingly, if at all. It is generally considered poor | |
| practice for an application to steal focus from other applications. |
(This will also require adding a .. app-notes: declaration just before the Notes header in the app documentation.)
| @@ -90,7 +90,7 @@ def show_about_dialog(self): | |||
| name_and_version += f" v{self.interface.version}" | |||
|
|
|||
| if self.interface.author is not None: | |||
| copyright = f"\n\nCopyright © {self.interface.author}" | |||
| @@ -385,3 +385,6 @@ def get_current_window(self): | |||
|
|
|||
| def set_current_window(self, window): | |||
| window._impl.native.makeKeyAndOrderFront(window._impl.native) | |||
|
|
|||
| def focus(self): | |||
| self.native.activateIgnoringOtherApps_(True) | |||
There was a problem hiding this comment.
The trailing underscore isn't needed here; it's a valid but historical syntax.
| self.native.activateIgnoringOtherApps_(True) | |
| self.native.activateIgnoringOtherApps(True) |
|
According to this page, Form.Activate doesn't actually activate a background app, but only flashes the taskbar button. So this is apparently a feature which can only be implemented on one platform, and is highly discouraged on that one. That seems inconsistent with the Toga philosophy. Other Microsoft sources: |
Even if Activate is the right API, the implementation here likely needs some modification - it should be using
Agreed this is a weird one. My counterargument would be that from a high level, the concept has a clear expression, which is why the subject has come up more than once. However bad an idea it may be, people ask for it; and I suspect being able to say "Here's the API, but be aware it won't work in a lot of scenarios and that's the platform's fault" is a lot more effective as line of argument than trying to convince people they don't want to use a feature (cf "image button" discussions). |
This PR introduces a new focus() method to the App class that lets you programmatically bring the application into focus (#3032). The implementation depends on the platform:
macOS: Uses activateIgnoringOtherApps_.
Windows: Calls Form.Activate().
GTK: Doesn't do anything (window managers manage focus).
Mobile/Web: Also no-op.
The method comes with a note in the docs to remind everyone that forcing focus isn't great UX and should only be used when necessary.
Why this change?
It solves the problem of needing a way to programmatically focus the app, especially for certain workflows or edge cases.
PR Checklist:
Tested the new feature.
Updated the documentation.
Read the CONTRIBUTING.md.
Promise to follow the code of conduct.